@doron050 any reason why you decided to create a constructor with a Transaction instead of just the Connection?

Now all the generated code does with the transaction is simply getting it's connection so that seems rather redundant

@doron050 any reason why you decided to create a constructor with a Transaction instead of just the Connection?

Now all the generated code does with the transaction is simply getting it's connection so that seems rather redundant

Hi @Kavantix, I'm following the sqlc best practice to maintain similarity to other SQLC plugins.
First, the generated code uses the transaction connection, but it also registers itself on the transaction object for the rollback/commit.

Please take a look at https://docs.sqlc.dev/en/stable/howto/transactions.html and see how they decided to implement transaction support. The main reason for this specific implementation is for the user to have full control over the transaction object and decide, based on their logic, when to rollback or commit.

To improve on this implementation, maybe we can do something like this:

var sqlQuery = new QuerySql(connectionString);
var transaction = sqlQuery.withTransaction();

That way, the initialization of the transaction will be inside the generated code, but again, the transaction object must be accessible from outside since the generated code can't control the commit and rollback logic.

WDYT?

@doron050 thanks for the detailed response, I see what you mean now with the registering of the transaction. The underlying libraries for C# work a lot different than the one for go with creating a new connection every time and having magic pooling and such so passing the connection seemed logical but after looking into it further having the transaction specifically is indeed required.

In the go version you indeed are in charge of the transaction yourself and you create a version of the queries struct by calling a method on the queries struct not constructing a new one.

The C# version perhaps should have a method on an existing queries object to attach a transaction to it which would create a new instance, that way you can use an injected queries class for all your queries just like in go you would use the existing queries object and get one out that will use the transaction.

There is however a slight problem with how the C# libraries work IMO, because if you want to manage the transaction yourself you suddenly also need to create the connection yourself which currently the generated code does on the fly.
So perhaps in addition to being able to make a queries object with a transaction we should also give it the power to start a new transaction.

Thus then you would use it like this:

class MyService(UsersSql userQueries, ConfigSql configQueries)
{
    void DoSomething() {
          using var transaction = userQueries.BeginTransaction();
          UsersSql userTransaction = userQueries.withTransaction(transaction);
          ConfigSql configTransaction = configQueries.withTransaction(transaction);

          await userTransaction.SomeQuery();
          await configTransaction.SomeOtherQuery();
          transaction.Commit()
    }

}

While writing this I realise though that there is another big difference with the go version. In the go version all queries are available on the same struct while the c# version generates a class per queries file. Which as you see above makes it slightly awkward to use the transaction across those queries.
Would it perhaps make sense to generate a 'helper' class that makes it easier to both begin the transaction and also in the correct way use the other QuerySql objects with or without a transaction?

@doron050 thanks for the detailed response, I see what you mean now with the registering of the transaction. The underlying libraries for C# work a lot different than the one for go with creating a new connection every time and having magic pooling and such so passing the connection seemed logical but after looking into it further having the transaction specifically is indeed required.

In the go version you indeed are in charge of the transaction yourself and you create a version of the queries struct by calling a method on the queries struct not constructing a new one.

The C# version perhaps should have a method on an existing queries object to attach a transaction to it which would create a new instance, that way you can use an injected queries class for all your queries just like in go you would use the existing queries object and get one out that will use the transaction.

There is however a slight problem with how the C# libraries work IMO, because if you want to manage the transaction yourself you suddenly also need to create the connection yourself which currently the generated code does on the fly. So perhaps in addition to being able to make a queries object with a transaction we should also give it the power to start a new transaction.

Thus then you would use it like this:

class MyService(UsersSql userQueries, ConfigSql configQueries)
{
    void DoSomething() {
          using var transaction = userQueries.BeginTransaction();
          UsersSql userTransaction = userQueries.withTransaction(transaction);
          ConfigSql configTransaction = configQueries.withTransaction(transaction);

          await userTransaction.SomeQuery();
          await configTransaction.SomeOtherQuery();
          transaction.Commit()
    }

}

While writing this I realise though that there is another big difference with the go version. In the go version all queries are available on the same struct while the c# version generates a class per queries file. Which as you see above makes it slightly awkward to use the transaction across those queries. Would it perhaps make sense to generate a 'helper' class that makes it easier to both begin the transaction and also in the correct way use the other QuerySql objects with or without a transaction?

Hi @Kavantix, Thanks for the long and detailed answers. After talking to @SockworkOrange about it, we decided on the following interface

class MyService(UsersSql userQueries, string connectionString)
{
    void DoSomething() {
          using var transaction = ExternalTransactionMethodCreation(connectionString);
          UsersSql userQueries = UsersSql.WithTransaction(transaction);
          ConfigSql configQueries = ConfigSql.WithTransaction(transaction);

          await userQueries.SomeQuery();
          await configQueries.SomeOtherQuery();
          transaction.Commit()
    }
}

This was decided because a connection object is not static. If we incorporate transaction creation into the code generation, users will have no control over the creation and termination of the connection object. That's probably why the sqlc go plugin did not implement the transaction creation in their version

Basically, we changed the ctor to the static WithTransaction to align the interface with the sqlc go version.